-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Platform-Guard Assertion Annotations for some known cases #52689
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Outdated
Show resolved
Hide resolved
14e5621
to
ef24082
Compare
I think there are a few more cases that could be probably annotated. For example one for consideration is SocketsHttpHandler.IsSupported and there was something in crypto too |
@buyaa-n One question just for my information: How could I use these guards to annotate the APIs which partly throw PNSE based on some condition, e.g. https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs#L32-L38 ? The methods using this one throw on ios/tvos (and a couple of related System.Net.NetworkInformation.Functional.Tests tests fail). |
Thanks, i remember |
Not sure, from the code in the link it doesn't look like it does platform check, seems doing capabilities check
That sound like its need UnsupportedOSPlatform on ios/tvos attributes (not guard attributes) |
9209a15
to
e2e3218
Compare
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not reuse _http3Enabled
for the guard. Add a new IsHttp3Supported
as @marek-safar suggests.
95d2aec
to
f4644e0
Compare
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Show resolved
Hide resolved
ff13a9d
to
8bdea14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
As part of #44922 now we have
Platform-Guard Assertion Annotation
attributes added to libraries and the Platform Compatibility Analyzer now supports them. So its time to useSupportedOSPlatformGuard
andUnsupportedOSPlatformGuard
whenever needed